Skip to content

feat: add get_by_ids, reading documents back by item ID - #21

Open
minseokpark-CL wants to merge 7 commits into
mainfrom
feat/get-by-ids
Open

minseokpark-CL wants to merge 7 commits into
mainfrom
feat/get-by-ids

Conversation

@minseokpark-CL

@minseokpark-CL minseokpark-CL commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

get_by_ids 추가 — item ID 로 문서 읽기

(English below.)

CryptoLabInc/envector-msa#2565 는 merge 됐고, 그 변경이 들어간 pyenvector 는 아직 release 되지 않았다. release 전 pyenvector 에서는 get_by_ids 가 NotImplementedError 를 내고 나머지 기능은 그대로 동작하므로, 이 PR 은 release 전에 main 에 넣을 수 있다. 아래 "의존성" 참고.

무엇이 달라지나

  • Envector.get_by_ids(ids, /, *, partition_name=None) 가 생긴다. add_texts / add_documents 가 돌려준 item ID(또는 검색 결과의 Document.id)로, 검색 없이 문서를 읽는다.
  • 살아 있는 item 마다 Document 하나를 ids 순서대로 돌려준다. Document.id 는 item ID 다. 없는 ID, 삭제된 ID, item ID 가 아닌 값은 에러 없이 빠진다(LangChain VectorStore.get_by_ids 의 규칙). item ID 는 양의 int 와 그 십진수 문자열뿐이고, bool·float(3.9, 3.0)·"3.0" 은 item ID 가 아니다 — int(...) 로 바꾸면 3.9 가 item 3 을 읽게 되므로(_readable_item_id). 같은 ID 가 여러 번 오면 한 번만 읽는다.
  • add_texts 가 끝나자마자 읽을 수 있고, delete 가 끝나자마자 읽히지 않는다. 인덱스 load 도 필요 없다. 벡터를 바꾸는 update_documents 를 기다리지 않고(await_completion=False) 부르면, 새 벡터가 검색 가능해질 때까지 get_by_ids 는 새 내용을 돌려주고 검색에서는 그 문서가 빠질 수 있다. 기본값(await_update)은 기다리므로 이 틈이 없다. docstring 에 적었다.
  • item ID 는 partition 안에서만 유일하다. named partition 에 넣은 문서는 같은 partition_name 으로 읽어야 한다.
  • 설치된 pyenvector 의 Index 에 get_by_ids 가 없으면 NotImplementedError 를 내고 pyenvector 를 올리라고 알린다. LangChain 이 get_by_ids 를 지원하지 않는 store 에서 내는 것과 같은 예외다. 같은 판단을 SDK_HAS_GET_BY_IDS 로 둔다.
  • 검색 결과를 Document 로 바꾸는 코드를 _stored_document 로 떼어 내 검색과 get_by_ids 가 같이 쓴다.
  • 표준 테스트: has_get_by_ids 는 SDK_HAS_GET_BY_IDS 를 따른다. tests/integration_tests/test_get_by_ids.py 도 같은 조건으로 skip 된다. test_add_documents_with_existing_ids 는 xfail 로 표시했다 — 이 테스트는 호출하는 쪽이 정한 id("foo")로 문서를 만들 수 있다고 가정하는데, item ID 는 서버가 발급한다.
  • README: Features 한 줄, Limitations 의 item ID 줄에 get_by_ids 를 넣고 받는 형식(반환된 문자열 또는 int)과 "UUID 같은 ID 는 get_by_ids 가 찾지 못한다" 를 적음, Limitations 에서 "get_by_ids unsupported" 를 빼고 partition 안내 한 줄과 "Index.get_by_ids 가 있는 pyenvector 가 필요하고, 그 전 버전에서는 NotImplementedError" 한 줄, "Fetch by ID" 예제.

의존성

  • get_by_ids 는 pyenvector 의 Index.get_by_ids 를 부른다. 이 메서드는 CryptoLabInc/envector-msa#2565 에서 들어갔고, 그 PR 은 2026-09-30 에 merge 됐다.
  • release 된 pyenvector 태그에는 Index.get_by_ids 가 없다: 1.6.2, 1.6.3-rc.1, 1.6.3-rc.2 모두 확인했다. PyPI 의 최신도 1.6.2 다.
  • pyproject.toml 과 README 의 요구 버전은 pyenvector >= 1.6.2 그대로다. 지금 설치하면 get_by_ids 호출은 NotImplementedError 로 끝나고, 나머지 기능은 그대로 동작한다.
  • 남은 순서: 그 변경이 들어간 pyenvector release → 요구 버전과 README 의 "Requires" 줄을 그 태그로 올리는 commit → release 된 SDK 로 integration test 실행.

알려진 문제 — 다음 PR 에서 수정

  • 이 PR 이전부터 있던 동작이다. delete, update_metadata, update_documents, upsert_documents 와 add_texts / add_documents 의 ids 는 ID 를 int(...) 로 바로 바꾼다. 그래서 True 는 item 1, 3.9 와 3.0 은 item 3 이 된다. 예를 들어 delete(ids=[3.9]) 는 item 3 을 지우고, add_texts(..., ids=[3.9]) 는 item 3 을 덮어쓴다.
  • 이 문제를 알고 있고, 이 PR 이 merge 된 바로 다음 PR(fix: refuse bool and float item IDs everywhere instead of coercing them #22, 이 PR 위에 쌓음)에서 고친다. 계획: ID 판별을 하나로 통일해 양의 int 와 그 십진수 문자열만 ID 로 보고 bool 과 float 는 ID 로 보지 않는다. 판별에 걸린 값의 처리는 메서드별 기존 방식을 따른다 — delete / update_* / upsert_documents 는 ValueError, add_texts 의 ids 는 새 행으로 삽입하고 UserWarning.
  • 이 PR 의 범위인 get_by_ids 추가와 분리해서, 기존 메서드의 동작 변경만 따로 리뷰할 수 있게 하려는 것이다.

검증

  • python -m pytest tests -m "not integration" — 35bfa33 에서 117 passed. 새 unit test: 요청 순서, 없는/item ID 가 아닌 ID 생략, 삭제된 문서 제외, partition 구분, 내용이 빈 live row, 복호화된 dict payload, text 가 null 인 envelope, 인덱스를 load 하지 않는 것, get_by_ids 가 없는 SDK 에서 NotImplementedError, bool·float·"3.0"·음수·0·공백·None·bytes·list 가 빠지고 SDK 호출도 없는 것(11가지), [3, " 2 ", "1", 3.9, True] 에서 3·2·1 만 읽는 것.
  • release 된 pyenvector 1.6.2 에서 SDK_HAS_GET_BY_IDS 는 False 이고, tests/integration_tests/test_get_by_ids.py 의 4개는 "installed pyenvector has no Index.get_by_ids" 로 skip 된다(652e876, 서버 없이 확인).
  • release 된 SDK 로는 integration test 를 아직 돌리지 않았다. release 뒤 요구 버전을 올리면서 돌린다.

Add get_by_ids — read documents back by item ID

CryptoLabInc/envector-msa#2565 is merged; no pyenvector release contains it yet. On a pyenvector without it get_by_ids raises NotImplementedError and everything else works as before, so this PR can land on main before that release. See "Dependency" below.

What changes

  • New Envector.get_by_ids(ids, /, *, partition_name=None): reads documents by the item IDs add_texts / add_documents return (or a search result's Document.id), without a search.
  • Returns one Document per live item, in the order of ids, with the item ID as Document.id. Unknown IDs, deleted IDs and values that are not item IDs are left out rather than raised, as LangChain's VectorStore.get_by_ids contract asks. An item ID is a positive int or its decimal string only; bool, float (3.9, 3.0) and "3.0" are not — converting them with int(...) would make 3.9 read item 3 (_readable_item_id). Repeated IDs are read once.
  • A document is readable as soon as add_texts returns and stops being readable as soon as delete returns. The index does not need to be loaded. After an update_documents that replaces the vector and is not awaited (await_completion=False), get_by_ids returns the new content while search may leave the document out until the new vector is searchable; the default (await_update) waits, so there is no gap. The docstring says so.
  • Item IDs are unique within a partition only; documents added to a named partition must be read with that partition_name.
  • If the installed pyenvector's Index has no get_by_ids, it raises NotImplementedError saying to upgrade pyenvector — the exception LangChain raises from a store without get_by_ids. The same check is exposed as SDK_HAS_GET_BY_IDS.
  • The code that turns a search hit into a Document moves to _stored_document, shared by search and get_by_ids.
  • Standard tests: has_get_by_ids follows SDK_HAS_GET_BY_IDS. tests/integration_tests/test_get_by_ids.py skips on the same condition. test_add_documents_with_existing_ids is marked xfail — it assumes a caller-chosen id ("foo") can be created, while item IDs are issued by the server.
  • README: one Features line; the Limitations line on item IDs now lists get_by_ids, names the accepted forms (the returned strings, or ints) and says get_by_ids never finds an ID such as a UUID; Limitations drops "get_by_ids unsupported" and gains one line on partitions and one line saying get_by_ids needs a pyenvector with Index.get_by_ids and raises NotImplementedError with an earlier one; a "Fetch by ID" example.

Dependency

  • get_by_ids calls pyenvector's Index.get_by_ids, added in CryptoLabInc/envector-msa#2565, which was merged on 2026-09-30.
  • No released pyenvector tag has Index.get_by_ids: checked 1.6.2, 1.6.3-rc.1 and 1.6.3-rc.2. The latest on PyPI is also 1.6.2.
  • pyproject.toml and the README still require pyenvector >= 1.6.2; installed today, a get_by_ids call ends in NotImplementedError and everything else works as before.
  • Remaining steps: release pyenvector with the change → a commit raising the requirement and the README "Requires" line to that tag → run the integration tests with the released SDK.

Known issue — fixed in the next PR

  • Pre-existing behaviour, not introduced here: delete, update_metadata, update_documents, upsert_documents and the ids of add_texts / add_documents turn an ID into int(...) directly, so True becomes item 1 and 3.9 or 3.0 becomes item 3. For example, delete(ids=[3.9]) deletes item 3, and add_texts(..., ids=[3.9]) overwrites item 3.
  • This is known and will be fixed in the PR right after this one is merged (fix: refuse bool and float item IDs everywhere instead of coercing them #22, stacked on this one). Plan: one ID check shared by all methods — only a positive int or its decimal string is an ID; bool and float are not. What happens to a rejected value follows each method's existing behaviour: ValueError from delete / update_* / upsert_documents, and a new row plus a UserWarning for the ids of add_texts.
  • It is kept out of this PR so the behaviour change to existing methods is reviewed on its own, separate from adding get_by_ids.

Verification

  • python -m pytest tests -m "not integration" — 117 passed on 35bfa33. New unit tests: request order, unknown and non-item IDs left out, deleted documents excluded, partitions, a live row with empty content, a decrypted dict payload, an envelope whose text is null, no index load, NotImplementedError on an SDK without get_by_ids, bool / float / "3.0" / negatives / zero / blank / None / bytes / list left out with no SDK call (11 cases), and [3, " 2 ", "1", 3.9, True] reading only 3, 2, 1.
  • With released pyenvector 1.6.2, SDK_HAS_GET_BY_IDS is False and the 4 tests in tests/integration_tests/test_get_by_ids.py skip with "installed pyenvector has no Index.get_by_ids" (652e876, checked without a server).
  • The integration tests have not been run with a released SDK yet; they run when the requirement is raised after the release.

🤖 Generated with Claude Code

@euphoria0-0 euphoria0-0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integration test 한번 실행 부탁드립니다!
릴리즈 없어도 가능하고 msa에서 get_by_ids 추가한 PR에서 직접 빌드한걸로 테스트해보면 됩니다

minseokpark-CL and others added 3 commits September 29, 2026 18:08
Envector.get_by_ids(ids, /, *, partition_name=None) returns a Document
for every id that names a live item, in request order. Non-item ids,
unknown ids and deleted ids are left out rather than raised, as
LangChain's contract asks. It uses pyenvector's Index.get_by_ids, which
reads stored metadata by item_id without a search, so a document is
readable as soon as add_texts returns and stops being readable as soon
as delete returns.

Item ids are unique within a partition only: a document added under a
named partition must be read with that partition_name, otherwise the
same number in the default partition is a different document. The
search result parsing is shared with get_by_ids through
_stored_document.

has_get_by_ids is now True; three of the four standard tests pass and
test_add_documents_with_existing_ids is xfail because a caller-chosen
id cannot be created. Requires a pyenvector with Index.get_by_ids; the
version pin is raised once that SDK release exists.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Index.get_by_ids already splits the ids at the server's per-call cap,
so the wrapper's own 10,000-id loop did nothing. get_by_ids now passes
all ids in one call, and the unit test for the wrapper-side split is
removed; the SDK's tests cover the split.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review on envector-msa #2565 asked that the difference be written where
users read it: a document is readable by id before a merge makes it
searchable, and right after an update the new content is readable while
search may still score the old vector. Both are expected.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@minseokpark-CL

Copy link
Copy Markdown
Contributor Author

@euphoria0-0 integration test 돌렸습니다. 실패 없습니다.

환경

  • langchain-envector: feat/get-by-ids @ a64d191
  • pyenvector: CryptoLabInc/envector-msa#2565 브랜치(feat/get-metadata-by-item-id @ 5e60cd4a)의 sdk/python 소스를 PYTHONPATH 로 사용
  • 서버: 같은 브랜치 @ 5e60cd4a 에서 빌드한 backend · endpoint · orchestrator 이미지로 docker compose 스택을 새로 띄워서 실행. shaper · compute 는 로컬에 있던 main-amd64 이미지(약 2주 전)를 썼습니다 — 이 PR 이 바꾸지 않은 서비스이고, 로컬에서 evi 소스 빌드가 되지 않아서입니다.

명령

ENVECTOR_ADDRESS=localhost:<port> ENVECTOR_KEY_PATH=<keys> ENVECTOR_KEY_ID=<key_id> \
  pytest tests/integration_tests -v

결과: 28 passed, 12 skipped, 3 xfailed (3m34s)

  • get_by_ids: test_get_by_ids.py 4개(insert 직후 읽힘 · delete 직후 사라짐, named partition — 각각 metadata 암호화 on/off) 와 표준 테스트 test_get_by_ids, test_get_by_ids_missing, test_add_documents_documents 모두 통과
  • xfail 3개는 기존 표시 그대로입니다: test_add_documents_with_existing_ids (호출자가 정한 id 로 생성 불가), test_deleting_documents / test_deleting_bulk_documents (delete 직후 top-k 에 잠깐 남음)
  • skip 12개는 async 표준 테스트입니다 (has_async=False)

…ut it

Index.get_by_ids ships in a pyenvector release that is not out yet,
while this package accepts pyenvector>=1.6.2. On 1.6.2 get_by_ids
failed with "'Index' object has no attribute 'get_by_ids'". Check the
installed SDK first and raise NotImplementedError, LangChain's usual
answer for an unsupported get_by_ids, saying to upgrade pyenvector.

SDK_HAS_GET_BY_IDS drives has_get_by_ids in the standard tests and
skips tests/integration_tests/test_get_by_ids.py, so the suites skip
instead of failing on an older SDK. README Limitations gets one line.
The pin and the README "Requires" line move once the release exists.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@minseokpark-CL
minseokpark-CL marked this pull request as ready for review October 1, 2026 05:16
minseokpark-CL and others added 2 commits October 1, 2026 15:21
get_by_ids turned ids into int(...) through the add_texts helper, so
True read item 1 and 3.9 or 3.0 read item 3 — a document the caller
did not ask for. pyenvector's own check rejects bool and float, but it
only ever saw the already-converted int.

_readable_item_id accepts a positive int or its decimal string and
nothing else; any other value names no item and is left out, as
LangChain's get_by_ids contract asks. Unit tests cover bool, float,
"3.0", negatives, zero, blanks, None, bytes and lists.

The same conversion in delete / update_* / upsert_documents and in the
ids of add_texts predates this PR and is fixed in the next one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Limitations line on item IDs listed delete / update / upsert /
add_documents but not get_by_ids, and said "any numeric id is taken to
be one of them", which get_by_ids no longer does for 3.0 or 3.9. Name
the accepted forms (the returned strings, or ints) and say that a
non-item ID such as a UUID is never found by get_by_ids.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tence

The docstring said that right after update_documents search "may still
score the old vector". It does not: a vector update deprecates the old
shard slot in the same transaction that writes the new metadata, so
until the new vector is searchable search leaves the document out
rather than ranking it by the old vector. update_documents waits for
that by default (await_update), so the gap exists only when it is called
with await_completion=False. Say that instead.

Also drop the list of accepted ID types from the first sentence: the
IDs callers pass back are the ones add_texts and search return, and the
next sentence already says anything that is not an item ID is left out.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@Kim-YeongHyeon Kim-YeongHyeon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 리뷰는 AI 에이전트가 전권 위임 하에 생성했습니다. 필수 부담당자 리뷰를 대체하지 않으며,
머지 판단은 사람 리뷰어에게 있습니다.

text = md_obj.get("text", "") if "_raw" not in md_obj else md_obj["_raw"]
metadata = md_obj.get("metadata", {}) if "_raw" not in md_obj else {}
if not text and not metadata:
doc, has_content = _stored_document(item)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검색 경로에서 Document 생성이 filter·score_threshold 판정보다 앞으로 옮겨져, refactor인데도 동작이 바뀝니다. 이전에는 threshold나 filter에서 걸러지던 비정상 payload가 이제는 pydantic ValidationError를 내고 검색 전체를 중단시킵니다. fake client로 score=0.1, score_threshold=0.5를 주고 확인한 결과입니다.

  • {"text":"x","metadata":null}: base [], head ValidationError
  • {"text":123,"metadata":{}}: base [], head ValidationError

get_by_ids도 같은 함수를 쓰므로, 다른 client가 쓴 행 하나만 형식이 어긋나도 호출 전체가 예외로 끝납니다(LangChain 계약은 raise하지 않는 것입니다). _stored_document가 text가 str이 아니거나 metadata가 dict가 아니면 None을 돌려주고, 두 호출처가 그 행을 건너뛰게 하는 방법을 제안합니다.

return value if value > 0 else None
if isinstance(value, str):
text = value.strip()
if text.isdecimal():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상한이 없어서 "9223372036854775808"이 isdecimal()과 SDK의 _item_id 검사를 모두 통과하고, proto의 repeated int64 item_ids에 들어가면서 ValueError: Value out of range가 납니다. 존재할 수 없는 ID인데 빠지지 않고 예외가 납니다. item_id > 2**63 - 1이면 None을 돌려주도록 해 주십시오.

또한 isdecimal()은 "3"(전각)이나 "٣" 같은 비ASCII 숫자도 받아 item 3을 읽습니다. #22에서 이 규칙을 모든 메서드에 적용할 계획이라면 text.isascii() and text.isdigit()로 범위를 정하고 docstring에 적어 두면 좋겠습니다.

Comment thread tests/test_vectorstore.py


@pytest.mark.parametrize(
"not_an_id", [True, False, 3.9, 3.0, "3.0", "-3", "0", " ", None, b"3", [3]]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문자열 "0", "-3"은 있지만 int 0, -3은 없습니다. _readable_item_id의 return value if value > 0 else None을 return value로 바꿔도 unit test 117개가 모두 통과합니다. 실제 SDK에서는 int 0이 EnvectorValidationError를 냅니다. int 0과 -3을 추가해 주십시오.

같은 맥락에서, 검색이 '{"text":"","metadata":{}}'를 건너뛰는지 확인하는 테스트도 없습니다(has_content를 항상 True로 바꿔도 통과합니다). 이 PR이 그 코드를 옮겼으므로 한 건 추가해 두면 좋겠습니다.

metadata=metadata,
id=str(doc_id) if doc_id is not None else None,
)
return doc, bool(text or metadata)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_content는 검색 경로만 쓰고 get_by_ids는 [0]으로 버립니다. Document만 돌려주고, 검색 쪽에서 if not (doc.page_content or doc.metadata): continue로 판단하면 튜플이 없어집니다. 위 검증 문제를 고치면서 반환값을 바꾸게 되면 함께 정리할 수 있습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants